Fix the inconsistencies in bl1_tbbr_image_descs[]

This patch fixes inconsistencies in bl1_tbbr_image_descs[]
and miscellaneous fixes in Firmware Update code.

Following are the changes:
* As part of the original FWU changes, a `copied_size`
  field was added to `image_info_t`. This was a subtle binary
  compatibility break because it changed the size of the
  `bl31_params_t` struct, which could cause problems if
  somebody used different versions of BL2 or BL31, one with
  the old `image_info_t` and one with the new version.
  This patch put the `copied_size` within the `image_desc_t`.
* EXECUTABLE flag is now stored in `ep_info.h.attr` in place
  of `image_info.h.attr`, associating it to an entrypoint.
* The `image_info.image_base` is only relevant for secure
  images that are copied from non-secure memory into secure
  memory. This patch removes initializing `image_base` for
  non secure images in the bl1_tbbr_image_descs[].
* A new macro `SET_STATIC_PARAM_HEAD` is added for populating
  bl1_tbbr_image_descs[].ep_info/image_info.h members statically.
  The version, image_type and image attributes are now
  populated using this new macro.
* Added PLAT_ARM_NVM_BASE and PLAT_ARM_NVM_SIZE to avoid direct
  usage of V2M_FLASH0_XXX in plat/arm/common/arm_bl1_fwu.c.
* Refactoring of code/macros related to SECURE and EXECUTABLE flags.

NOTE: PLATFORM PORTS THAT RELY ON THE SIZE OF `image_info_t`
      OR USE the "EXECUTABLE" BIT WITHIN `image_info_t.h.attr`
      OR USE THEIR OWN `image_desc_t` ARRAY IN BL1, MAY BE
      BROKEN BY THIS CHANGE. THIS IS CONSIDERED UNLIKELY.

Change-Id: Id4e5989af7bf0ed263d19d3751939da1169b561d

diff --git a/bl1/bl1_context_mgmt.c b/bl1/bl1_context_mgmt.c
index 6355190e5579cf3e0cd96f1542117d62bf97e233..bd40608bc789fdc79361796fe4898a69439c6262 100644 (file)
--- a/bl1/bl1_context_mgmt.c
+++ b/bl1/bl1_context_mgmt.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -74,7 +74,7 @@ void bl1_prepare_next_image(unsigned int image_id)
        next_bl_ep = &image_desc->ep_info;
 
        /* Get the image security state. */
-       security_state = GET_SEC_STATE(next_bl_ep->h.attr);
+       security_state = GET_SECURITY_STATE(next_bl_ep->h.attr);
 
        /* Setup the Secure/Non-Secure context if not done already. */
        if (!cm_get_context(security_state))

diff --git a/bl1/bl1_fwu.c b/bl1/bl1_fwu.c
index 80ce831a515c9b74ce79738185d1b4d74eee18a6..f3338051d9171feea9489e46ea3cbfc81b5c77ba 100644 (file)
--- a/bl1/bl1_fwu.c
+++ b/bl1/bl1_fwu.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -135,8 +135,8 @@ static int bl1_fwu_image_copy(unsigned int image_id,
        }
 
        /* Only Normal world is allowed to copy a Secure image. */
-       if ((GET_SEC_STATE(flags) == SECURE) ||
-               (GET_SEC_STATE(image_desc->ep_info.h.attr) == NON_SECURE)) {
+       if ((GET_SECURITY_STATE(flags) == SECURE) ||
+           (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == NON_SECURE)) {
                WARN("BL1-FWU: Copy not allowed for Non-Secure "
                         "image from Secure-world\n");
                return -EPERM;
@@ -156,10 +156,10 @@ static int bl1_fwu_image_copy(unsigned int image_id,
                 * If last block is more than expected then
                 * clip the block to the required image size.
                 */
-               if (image_desc->image_info.copied_size + block_size >
+               if (image_desc->copied_size + block_size >
                         image_desc->image_info.image_size) {
                        block_size = image_desc->image_info.image_size -
-                               image_desc->image_info.copied_size;
+                               image_desc->copied_size;
                        WARN("BL1-FWU: Copy argument block_size > remaining image size."
                                " Clipping block_size\n");
                }
@@ -173,13 +173,13 @@ static int bl1_fwu_image_copy(unsigned int image_id,
                INFO("BL1-FWU: Continuing image copy in blocks\n");
 
                /* Copy image for given block size. */
-               base_addr += image_desc->image_info.copied_size;
-               image_desc->image_info.copied_size += block_size;
+               base_addr += image_desc->copied_size;
+               image_desc->copied_size += block_size;
                memcpy((void *)base_addr, (const void *)image_src, block_size);
                flush_dcache_range(base_addr, block_size);
 
                /* Update the state if last block. */
-               if (image_desc->image_info.copied_size ==
+               if (image_desc->copied_size ==
                                image_desc->image_info.image_size) {
                        image_desc->state = IMAGE_STATE_COPIED;
                        INFO("BL1-FWU: Image copy in blocks completed\n");
@@ -234,7 +234,7 @@ static int bl1_fwu_image_copy(unsigned int image_id,
                        INFO("BL1-FWU: Started image copy in blocks\n");
                }
 
-               image_desc->image_info.copied_size = block_size;
+               image_desc->copied_size = block_size;
        }
 
        return 0;
@@ -257,14 +257,14 @@ static int bl1_fwu_image_auth(unsigned int image_id,
        if (!image_desc)
                return -EPERM;
 
-       if (GET_SEC_STATE(flags) == SECURE) {
+       if (GET_SECURITY_STATE(flags) == SECURE) {
                if (image_desc->state != IMAGE_STATE_RESET) {
                        WARN("BL1-FWU: Authentication from secure world "
                                "while in invalid state\n");
                        return -EPERM;
                }
        } else {
-               if (GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE) {
+               if (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE) {
                        if (image_desc->state != IMAGE_STATE_COPIED) {
                                WARN("BL1-FWU: Authentication of secure image "
                                        "from non-secure world while not in copied state\n");
@@ -369,10 +369,10 @@ static int bl1_fwu_image_execute(unsigned int image_id,
         * Image is NOT in AUTHENTICATED state.
         */
        if ((!image_desc) ||
-               (GET_SEC_STATE(flags) == SECURE) ||
-               (GET_SEC_STATE(image_desc->ep_info.h.attr) == NON_SECURE) ||
-               (GET_EXEC_STATE(image_desc->image_info.h.attr) == NON_EXECUTABLE) ||
-               (image_desc->state != IMAGE_STATE_AUTHENTICATED)) {
+           (GET_SECURITY_STATE(flags) == SECURE) ||
+           (GET_SECURITY_STATE(image_desc->ep_info.h.attr) == NON_SECURE) ||
+           (EP_GET_EXE(image_desc->ep_info.h.attr) == NON_EXECUTABLE) ||
+           (image_desc->state != IMAGE_STATE_AUTHENTICATED)) {
                WARN("BL1-FWU: Execution not allowed due to invalid state/args\n");
                return -EPERM;
        }
@@ -402,7 +402,7 @@ static register_t bl1_fwu_image_resume(register_t image_param,
 {
        image_desc_t *image_desc;
        unsigned int resume_sec_state;
-       unsigned int caller_sec_state = GET_SEC_STATE(flags);
+       unsigned int caller_sec_state = GET_SECURITY_STATE(flags);
 
        /* Get the image descriptor for last executed secure image id. */
        image_desc = bl1_plat_get_image_desc(sec_exec_image_id);
@@ -417,8 +417,8 @@ static register_t bl1_fwu_image_resume(register_t image_param,
                assert(image_desc);
        }
 
-       assert(GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE);
-       assert(GET_EXEC_STATE(image_desc->image_info.h.attr) == EXECUTABLE);
+       assert(GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE);
+       assert(EP_GET_EXE(image_desc->ep_info.h.attr) == EXECUTABLE);
 
        if (caller_sec_state == SECURE) {
                assert(image_desc->state == IMAGE_STATE_EXECUTED);
@@ -458,7 +458,7 @@ static int bl1_fwu_sec_image_done(void **handle, unsigned int flags)
        image_desc_t *image_desc;
 
        /* Make sure caller is from the secure world */
-       if (GET_SEC_STATE(flags) == NON_SECURE) {
+       if (GET_SECURITY_STATE(flags) == NON_SECURE) {
                WARN("BL1-FWU: Image done not allowed from normal world\n");
                return -EPERM;
        }
@@ -468,8 +468,8 @@ static int bl1_fwu_sec_image_done(void **handle, unsigned int flags)
 
        /* image_desc must correspond to a valid secure executing image */
        assert(image_desc);
-       assert(GET_SEC_STATE(image_desc->ep_info.h.attr) == SECURE);
-       assert(GET_EXEC_STATE(image_desc->image_info.h.attr) == EXECUTABLE);
+       assert(GET_SECURITY_STATE(image_desc->ep_info.h.attr) == SECURE);
+       assert(EP_GET_EXE(image_desc->ep_info.h.attr) == EXECUTABLE);
        assert(image_desc->state == IMAGE_STATE_EXECUTED);
 
        /* Update the flags. */

diff --git a/bl1/tbbr/tbbr_img_desc.c b/bl1/tbbr/tbbr_img_desc.c
index 42de8517f8c02eccab9c97df6f866d10b7bf64ed..7651f1c041eb5e5e30a7d6c7d353c8cf71f2de72 100644 (file)
--- a/bl1/tbbr/tbbr_img_desc.c
+++ b/bl1/tbbr/tbbr_img_desc.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -35,42 +35,46 @@
 image_desc_t bl1_tbbr_image_descs[] = {
     {
            .image_id = FWU_CERT_ID,
-           .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
+           SET_STATIC_PARAM_HEAD(image_info, PARAM_IMAGE_BINARY,
+                   VERSION_1, image_info_t, 0),
            .image_info.image_base = BL2_BASE,
-           .ep_info.h.attr = SET_SEC_STATE(SECURE),
+           SET_STATIC_PARAM_HEAD(ep_info, PARAM_IMAGE_BINARY,
+                   VERSION_1, entry_point_info_t, SECURE),
     },
 #if NS_BL1U_BASE
     {
            .image_id = NS_BL1U_IMAGE_ID,
-           .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),
-           .image_info.image_base = NS_BL1U_BASE,
-           .ep_info.h.attr = SET_SEC_STATE(NON_SECURE),
+           SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+                   VERSION_1, entry_point_info_t, NON_SECURE | EXECUTABLE),
            .ep_info.pc = NS_BL1U_BASE,
     },
 #endif
 #if SCP_BL2U_BASE
     {
            .image_id = SCP_BL2U_IMAGE_ID,
-           .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
+           SET_STATIC_PARAM_HEAD(image_info, PARAM_IMAGE_BINARY,
+                   VERSION_1, image_info_t, 0),
            .image_info.image_base = SCP_BL2U_BASE,
-           .ep_info.h.attr = SET_SEC_STATE(SECURE),
+           SET_STATIC_PARAM_HEAD(ep_info, PARAM_IMAGE_BINARY,
+                   VERSION_1, entry_point_info_t, SECURE),
     },
 #endif
 #if BL2U_BASE
     {
            .image_id = BL2U_IMAGE_ID,
-           .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),
+           SET_STATIC_PARAM_HEAD(image_info, PARAM_EP,
+                   VERSION_1, image_info_t, 0),
            .image_info.image_base = BL2U_BASE,
-           .ep_info.h.attr = SET_SEC_STATE(SECURE),
+           SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+                   VERSION_1, entry_point_info_t, SECURE | EXECUTABLE),
            .ep_info.pc = BL2U_BASE,
     },
 #endif
 #if NS_BL2U_BASE
     {
            .image_id = NS_BL2U_IMAGE_ID,
-           .image_info.h.attr = SET_EXEC_STATE(NON_EXECUTABLE),
-           .image_info.image_base = NS_BL2U_BASE,
-           .ep_info.h.attr = SET_SEC_STATE(NON_SECURE),
+           SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,
+                   VERSION_1, entry_point_info_t, NON_SECURE),
     },
 #endif
            BL2_IMAGE_DESC,

diff --git a/include/common/bl_common.h b/include/common/bl_common.h
index e5e6717b610f85b4827c0ff4a765391080d37627..f13dc316cfd01434f2e8fa398e7da4111291dfaa 100644 (file)
--- a/include/common/bl_common.h
+++ b/include/common/bl_common.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013-2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2013-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -53,28 +53,12 @@
 #define ENTRY_POINT_INFO_ARGS_OFFSET   0x18
 
 /* The following are used to set/get image attributes. */
-#define EXECUTABLE                     (0x1)
-#define NON_EXECUTABLE                 (0x0)
-#define PARAM_EP_EXECUTE_MASK          (0x1)
-#define PARAM_EP_EXECUTE_SHIFT         (0x1)
 #define PARAM_EP_SECURITY_MASK         (0x1)
-#define PARAM_EP_SECURITY_SHIFT                (0x0)
 
 #define GET_SECURITY_STATE(x) (x & PARAM_EP_SECURITY_MASK)
 #define SET_SECURITY_STATE(x, security) \
                        ((x) = ((x) & ~PARAM_EP_SECURITY_MASK) | (security))
 
-#define GET_EXEC_STATE(x)    \
-    (((x) >> PARAM_EP_EXECUTE_SHIFT) & PARAM_EP_EXECUTE_MASK)
-
-#define SET_EXEC_STATE(x)    \
-    (((x) & PARAM_EP_EXECUTE_MASK) << PARAM_EP_EXECUTE_SHIFT)
-
-#define GET_SEC_STATE(x)    \
-    (((x) >> PARAM_EP_SECURITY_SHIFT) & PARAM_EP_SECURITY_MASK)
-
-#define SET_SEC_STATE(x)    \
-    (((x) & PARAM_EP_SECURITY_MASK) << PARAM_EP_SECURITY_SHIFT)
 
 /*
  * The following are used for image state attributes.
@@ -99,11 +83,17 @@
 #define EP_GET_ST(x) (x & EP_ST_MASK)
 #define EP_SET_ST(x, ee) ((x) = ((x) & ~EP_ST_MASK) | (ee))
 
-#define PARAM_EP     0x01
-#define PARAM_IMAGE_BINARY  0x02
-#define PARAM_BL31       0x03
+#define EP_EXE_MASK    0x8
+#define NON_EXECUTABLE 0x0
+#define EXECUTABLE     0x8
+#define EP_GET_EXE(x) (x & EP_EXE_MASK)
+#define EP_SET_EXE(x, ee) ((x) = ((x) & ~EP_EXE_MASK) | (ee))
+
+#define PARAM_EP               0x01
+#define PARAM_IMAGE_BINARY     0x02
+#define PARAM_BL31             0x03
 
-#define VERSION_1              0x01
+#define VERSION_1      0x01
 
 #define INVALID_IMAGE_ID               (0xFFFFFFFF)
 
@@ -114,6 +104,14 @@
        (_p)->h.attr = (uint32_t)(_attr) ; \
        } while (0)
 
+/* Following is used for populating structure members statically. */
+#define SET_STATIC_PARAM_HEAD(_p, _type, _ver, _p_type, _attr) \
+       ._p.h.type = (uint8_t)(_type), \
+       ._p.h.version = (uint8_t)(_ver), \
+       ._p.h.size = (uint16_t)sizeof(_p_type), \
+       ._p.h.attr = (uint32_t)(_attr)
+
+
 /*******************************************************************************
  * Constants to indicate type of exception to the common exception handler.
  ******************************************************************************/
@@ -224,7 +222,6 @@ typedef struct image_info {
        param_header_t h;
        uintptr_t image_base;   /* physical address of base of image */
        uint32_t image_size;    /* bytes read from image file */
-       uint32_t copied_size;   /* image size copied in blocks */
 } image_info_t;
 
 /*****************************************************************************
@@ -238,6 +235,7 @@ typedef struct image_desc {
         * Refer IMAGE_STATE_XXX defined above.
         */
        unsigned int state;
+       uint32_t copied_size;   /* image size copied in blocks */
        image_info_t image_info;
        entry_point_info_t ep_info;
 } image_desc_t;

diff --git a/include/plat/arm/board/common/board_arm_def.h b/include/plat/arm/board/common/board_arm_def.h
index db2a8dfb062e207655faf483dfdd9a943642c230..70f6b5b5a70b8ed77e046c274795b157c536887e 100644 (file)
--- a/include/plat/arm/board/common/board_arm_def.h
+++ b/include/plat/arm/board/common/board_arm_def.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -143,5 +143,8 @@
 #define PLAT_ARM_FIP_BASE              V2M_FLASH0_BASE
 #define PLAT_ARM_FIP_MAX_SIZE          V2M_FLASH0_SIZE
 
+#define PLAT_ARM_NVM_BASE              V2M_FLASH0_BASE
+#define PLAT_ARM_NVM_SIZE              V2M_FLASH0_SIZE
+
 
 #endif /* __BOARD_ARM_DEF_H__ */

diff --git a/include/plat/arm/common/arm_def.h b/include/plat/arm/common/arm_def.h
index 8d753637c47f5cef87e0ac23fc5adba9dc5948f6..95cb64895bc2ad68bdd318fcc680f8118cca06b0 100644 (file)
--- a/include/plat/arm/common/arm_def.h
+++ b/include/plat/arm/common/arm_def.h
@@ -299,7 +299,7 @@
 #define BL2U_BASE                      BL2_BASE
 #define BL2U_LIMIT                     BL31_BASE
 #define NS_BL2U_BASE                   ARM_NS_DRAM1_BASE
-#define NS_BL1U_BASE                   (V2M_FLASH0_BASE + 0x03EB8000)
+#define NS_BL1U_BASE                   (PLAT_ARM_NVM_BASE + 0x03EB8000)
 
 /*
  * ID of the secure physical generic timer interrupt used by the TSP.

diff --git a/include/plat/common/common_def.h b/include/plat/common/common_def.h
index 916720c5fbbe74b9f521c3b5b1aaa4c643757488..9fac9fa22131c648f82f4251985ad5eee05ac758 100644 (file)
--- a/include/plat/common/common_def.h
+++ b/include/plat/common/common_def.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -72,11 +72,12 @@
 
 #define BL2_IMAGE_DESC {                               \
        .image_id = BL2_IMAGE_ID,                       \
-       .image_info.h.version = VERSION_1,              \
-       .image_info.h.attr = SET_EXEC_STATE(EXECUTABLE),\
+       SET_STATIC_PARAM_HEAD(image_info, PARAM_EP,     \
+               VERSION_1, image_info_t, 0),            \
        .image_info.image_base = BL2_BASE,              \
-       .ep_info.h.attr = SET_SEC_STATE(SECURE),        \
-       .ep_info.pc = BL2_BASE                          \
+       SET_STATIC_PARAM_HEAD(ep_info, PARAM_EP,        \
+               VERSION_1, entry_point_info_t, SECURE | EXECUTABLE),\
+       .ep_info.pc = BL2_BASE,                         \
 }
 
 #endif /* __COMMON_DEF_H__ */

diff --git a/plat/arm/common/arm_bl1_fwu.c b/plat/arm/common/arm_bl1_fwu.c
index 9a0d93ad60d5e1d699cd8e891f798ec9cc91263d..2a18d34130aabb60e90d1b7e905a39918b8b2c42 100644 (file)
--- a/plat/arm/common/arm_bl1_fwu.c
+++ b/plat/arm/common/arm_bl1_fwu.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved.
+ * Copyright (c) 2015-2016, ARM Limited and Contributors. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
@@ -33,11 +33,12 @@
 #include <debug.h>
 #include <errno.h>
 #include <plat_arm.h>
+#include <platform_def.h>
 #include <tbbr_img_desc.h>
 
 
 /* Struct to keep track of usable memory */
-typedef struct bl1_mem_info{
+typedef struct bl1_mem_info {
        uintptr_t mem_base;
        unsigned int mem_size;
 } bl1_mem_info_t;
@@ -58,8 +59,8 @@ bl1_mem_info_t fwu_addr_map_non_secure[] = {
                .mem_size = ARM_NS_DRAM1_SIZE
        },
        {
-               .mem_base = V2M_FLASH0_BASE,
-               .mem_size = V2M_FLASH0_SIZE
+               .mem_base = PLAT_ARM_NVM_BASE,
+               .mem_size = PLAT_ARM_NVM_SIZE
        },
        {
                .mem_size = 0
@@ -79,7 +80,7 @@ int bl1_plat_mem_check(uintptr_t mem_base,
        /*
         * Check the given image source and size.
         */
-       if (GET_SEC_STATE(flags) == SECURE)
+       if (GET_SECURITY_STATE(flags) == SECURE)
                mmap = fwu_addr_map_secure;
        else
                mmap = fwu_addr_map_non_secure;

diff --git a/plat/arm/common/arm_common.mk b/plat/arm/common/arm_common.mk
index 2647f043a379c92e6b944a26580b3814ecc164ea..425e0d367a4620546075b53791a5bde4808e3520 100644 (file)
--- a/plat/arm/common/arm_common.mk
+++ b/plat/arm/common/arm_common.mk
@@ -129,8 +129,8 @@ ifneq (${TRUSTED_BOARD_BOOT},0)
 
     PLAT_INCLUDES      +=      -Iinclude/bl1/tbbr
 
-    BL1_SOURCES                +=      ${AUTH_SOURCES}                 \
-                               bl1/tbbr/tbbr_img_desc.c        \
+    BL1_SOURCES                +=      ${AUTH_SOURCES}                                 \
+                               bl1/tbbr/tbbr_img_desc.c                        \
                                plat/arm/common/arm_bl1_fwu.c
 
     BL2_SOURCES                +=      ${AUTH_SOURCES}